Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Image config option added: rebuildOnContextPathChanges #98

Merged

Conversation

consideRatio
Copy link
Member

I'm working to make BinderHub's CI system easy to understand and performant, current tests take at least 13 minutes to complete, but this can change by not rebuilding the binderhub Docker image on any change as we currently do.

This PR adds the feature to allow the configuration option for indvidual images, which I named to rebuildOnContextPathChanges. Chartpress will assume this to be True by default, but if it is set to false, it will stop including the image's docker build contextPath as something that should be monitored for a rebuild indication whenever something changes.

With this disabled, one need to manually add paths that should be monitored to trigger changes though. In the jupyterhub/binderhub repo, I tested the following chartpress.yaml configuration which worked well.

# For a reference on this configuration, see the chartpress README file.
# ref: https://github.com/jupyterhub/chartpress
#
# NOTE: All paths will be set relative to this file's location, which is in the
#       helm-chart folder.
charts:
  - name: binderhub
    imagePrefix: jupyterhub/k8s-
    repo:
      git: jupyterhub/helm-chart
      published: https://jupyterhub.github.io/helm-chart
    resetTag: local
    resetVersion: 0.2.0
    images:
      image-cleaner:
        valuesPath: imageCleaner.image
      binderhub:
        # We will not use the default build contextPath, and must therefore
        # specify the dockerfilePath explicitly.
        dockerfilePath: images/binderhub/Dockerfile
        # Context to send to docker build for use by the Dockerfile. We pass the
        # root folder in order to allow the image to access and build the python
        # package.
        contextPath: ..
        # To avoid chartpress to react to changes in documentation and other
        # things, we ask it to not trigger on changes to the contextPath, which
        # means we manually should add paths rebuild should be triggered on
        rebuildOnContextPathChanges: false
        # We manually specify the paths which chartpress should monitor for
        # changes that should trigger a rebuild of this image.
        paths:
          - images/binderhub/requirements.txt
          - ../setup.py
          - ../package.json
          - ../binderhub
        valuesPath: image

The old one didn't specify paths or had the rebuildOnContextPathChanges: false configuration, but ended up rebuilding the Dockerfile on any change because the contextPath was set to the root of the repo, in order to be able to copy the binderhub folder with the Python package.

I've documented it in the Readme, but didn't add a test other than test it myself locally. I think it should be fine to leave untested as it is a quite simple feature.

Copy link
Member

@sgibson91 sgibson91 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants